Add config key to set the file creation permissions #537

fortizc · 2024-12-13T23:36:04Z

This PR defines a new config key called create_files_mode to set the default permission used when a file is created.
Currently, Oil uses 644 (420 in decimal) but, at least on Linux most of the tools like (n)vim, touch, or even a simple > filename by default uses 664 (436 decimal) so, with this option the user is able to set the default value.

stevearc · 2024-12-21T12:06:46Z

lua/oil/config.lua

+  -- Set the default mode to create files (:help oil-file_creation)
+  create_files_mode = 420,


We don't expect most users to need to modify this, so let's move it down to below the view_options.

If we're adding the ability to adjust the mode of the created files, we should do the same for directories. We could use two options for this new_file_mode and new_dir_mode.

Since most people are used to thinking about file permissions in octal, the config option should be octal and we should automatically convert it.

ok, sounds good

stevearc · 2024-12-21T12:07:34Z

lua/oil/config.lua

@@ -408,6 +412,8 @@ M.setup = function(opts)
  if opts.preview and not opts.confirmation then
    new_conf.confirmation = vim.tbl_deep_extend("keep", opts.preview, default_config.confirmation)
  end
+
+  M.file_mode = opts.create_files_mode


Why are we renaming this here?

I will fix it

stevearc · 2024-12-21T12:15:51Z

doc/oil.txt

+--------------------------------------------------------------------------------
+FILE CREATION                                                  *oil-file_creation*
+
+
+Oil defines `create_files_mode` to set the permission for new files. The mode
+follows the Posix numeric standard which use octal notation (base-8), however
+the variable must be set in decimal.
+
+    00700 user (file owner) has read, write, and execute permission
+    00400 user has read permission
+    00200 user has write permission
+    00100 user has execute permission
+    00070 group has read, write, and execute permission
+    00040 group has read permission
+    00020 group has write permission
+    00010 group has execute permission
+    00007 others have read, write, and execute permission
+    00004 others have read permission
+    00002 others have write permission
+    00001 others have execute permission
+    (from the open (2) man pages)
+
+An easy way to provide the right value for `create_files_mode` is using
+`tonumber(e [, base])` function with `base = 8`, for example:
+>lua
+    create_files_mode = tonumber("0644", 8)
+
+The default value is 420 in decimal or 0644 in octal
+
+


I actually don't think we need this section. It's explaining:

what the octal file permissions mean

how to convert octal to decimal

the default value

Point 3 is already documented in the options section. Point 2 is obviated if we accept the permission in octal. And for point 1 I would argue that if you don't know what octal file permissions are, you probably shouldn't be changing this config value and I don't think the oil docs are the correct place to be learning about them.

Ok, then I will remove it

stevearc · 2025-01-03T04:44:11Z

lua/oil/fs.lua

 ---@param mode? integer
 M.mkdirp = function(dir, mode)
-  mode = mode or 493
  local mod = ""


According to the docs, uv.fs_mkdir expects a non-nil integer as the second argument. Did you intend to replace this with the value in the config file?

Yes, the idea is to use the config value, and now we have a default one, so it's always available

Then you'll need to update the function signature to make mode non-optional, and update the call here to pass the mode

oil.nvim/lua/oil/adapters/ssh.lua

Line 371 in c12fad2

fs.mkdirp(tmpdir)

stevearc · 2025-01-03T04:48:16Z

doc/oil.txt

+      -- Set the default mode to create files
+      new_files_mode = 644,
+      -- Set the default mode to create folders
+      new_folder_mode = 755,


I am fully aware of how this sounds, and I'm really sorry to be this pedantic, but "folder" is Windows terminology. Typically in Unix contexts we would refer to these as "directories". Elsewhere in this plugin we always refer to these as directories. For consistency sake, could we rename this to new_dir_mode?

Add config key to set the file creation permissions Rename file_mode to new_file_mode Add new_folder_mode to store the default folder creation mode Remove doc about new_file_mode Fix how the folder and file mode are added to config Rename new_folder_mode to new_dir_mode

stevearc · 2025-01-05T21:25:53Z

lua/oil/fs.lua

 ---@param mode? integer
 M.mkdirp = function(dir, mode)
-  mode = mode or 493
  local mod = ""


Then you'll need to update the function signature to make mode non-optional, and update the call here to pass the mode

oil.nvim/lua/oil/adapters/ssh.lua

Line 371 in c12fad2

fs.mkdirp(tmpdir)

stevearc · 2025-01-05T21:28:02Z

lua/oil/config.lua

+  new_conf.new_file_mode = tonumber(tostring(new_conf.new_file_mode), 8)
+  new_conf.new_dir_mode = tonumber(tostring(new_conf.new_dir_mode), 8)


nit: you don't actually need the tostring() here. tonumber will convert an integer or a string the same way.

Not sure about it, i try the following code:

local x = 493 print(tonumber(x, 8))

and I get the following error:

$ lua test.lua lua: test.lua:2: bad argument #1 to 'tonumber' (string expected, got number) stack traceback: [C]: in function 'tonumber' test.lua:2: in main chunk [C]: in ?

I'm not a Lua expert, in fact this is my first productive code in Lua, so maybe I'm ignoring something

the second argument to tonumber is the base to interpret the number as. 493 is not a valid octal number. If you do tonumber(755, 8) it prints out 493 as expected

Again the same:

$ lua -e "print(tonumber(755, 8))" lua: (command line):1: bad argument #1 to 'tonumber' (string expected, got number) stack traceback: [C]: in function 'tonumber' (command line):1: in main chunk [C]: in ?

Maybe this has something to do with my interpreter? to run this examples I used the standalone interpreter:

$ lua Lua 5.4.6 Copyright (C) 1994-2023 Lua.org, PUC-Rio

Oh interesting, must be a difference between LuaJIT and Lua then (or 5.1 and 5.4). I retract the nit

github-actions bot requested a review from stevearc December 13, 2024 23:36

stevearc requested changes Dec 21, 2024

View reviewed changes

github-actions bot requested a review from stevearc December 26, 2024 00:38

fortizc force-pushed the feature/file-modes branch 2 times, most recently from e96ca6f to d91e574 Compare December 26, 2024 00:52

stevearc requested changes Jan 3, 2025

View reviewed changes

fortizc force-pushed the feature/file-modes branch from d91e574 to 17ae7b0 Compare January 5, 2025 19:24

github-actions bot requested a review from stevearc January 5, 2025 19:24

stevearc requested changes Jan 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add config key to set the file creation permissions #537

Add config key to set the file creation permissions #537

fortizc commented Dec 13, 2024

stevearc Dec 21, 2024

stevearc Dec 21, 2024

stevearc Dec 21, 2024

fortizc Dec 24, 2024

stevearc Dec 21, 2024

fortizc Dec 24, 2024

stevearc Dec 21, 2024

fortizc Dec 24, 2024

stevearc Jan 3, 2025

fortizc Jan 5, 2025

stevearc Jan 5, 2025

stevearc Jan 3, 2025

fortizc Jan 5, 2025

stevearc Jan 5, 2025

stevearc Jan 5, 2025

fortizc Jan 5, 2025

stevearc Jan 6, 2025

fortizc Jan 6, 2025

stevearc Jan 7, 2025

		-- Set the default mode to create files (:help oil-file_creation)
		create_files_mode = 420,

		new_conf.new_file_mode = tonumber(tostring(new_conf.new_file_mode), 8)
		new_conf.new_dir_mode = tonumber(tostring(new_conf.new_dir_mode), 8)

Add config key to set the file creation permissions #537

Are you sure you want to change the base?

Add config key to set the file creation permissions #537

Conversation

fortizc commented Dec 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment